Skip to content

MCP application profile: custom content resources via static mcpResources#1613

Merged
kriszyp merged 7 commits into
v5.1from
feat/mcp-custom-resources-1609
Jul 7, 2026
Merged

MCP application profile: custom content resources via static mcpResources#1613
kriszyp merged 7 commits into
v5.1from
feat/mcp-custom-resources-1609

Conversation

@kylebernhardy

@kylebernhardy kylebernhardy commented Jul 6, 2026

Copy link
Copy Markdown
Member

Fixes #1609.

Summary

Component authors can now expose arbitrary resources/read-able content — the issue's docs-server case: every documentation page as an MCP resource — via static mcpResources on a Resource class, completing the custom-surface triad with mcpTools (#622) and mcpPrompts (#1349 §3.5):

DocsPages.mcpResources = [
	{ uri: 'docs:///index', name: 'docs index', description: '…', mimeType: 'text/markdown', method: 'readIndex' },
	{ uriTemplate: 'docs:///{+path}', name: 'docs page', method: 'readPage',
	  completions: { path: ['guides/install.md', ] } },
];
  • Registry (components/mcp/customResourceRegistry.ts, mirrors promptRegistry): fixed URIs list in resources/list, templates in resources/templates/list; {name} matches one segment, {+name} crosses segments (RFC-6570-lite, matching how clients expand templates); fixed match wins over templates.
  • Reads dispatch the named instance method on the live registry class (same rationale as mcpTools: the exported resources.js subclass carries the author's access control; RBAC is delegated to the Resource). Returns: string → text, {text}/{blob,mimeType} pass through, other objects serialize as JSON. Author errors are sanitized (raw error only in the server log).
  • Completion (completion/complete) serves author-declared per-parameter values, selected by the client's ref.uri.
  • list_changed: the per-session diff snapshot now includes templates, so template-only changes notify.

Descriptor scheme change (issue's "minor, related", user-approved): exported-Resource descriptors now list under harper+rest:// — the spec reserves https:// for resources a client can fetch from the web, and these resolve in-process. Legacy http(s):// URIs continue to read and subscribe (back-compat). Custom mcpResources may not use reserved schemes (harper:, harper+rest:, http(s):) — prevents shadowing built-ins like harper://schema/....

Latent registration gap fixed en route: component entry loading is asynchronous past the boot awaits (chokidar's initial scan completes after loadComponentDirectories resolves), so a tableless component's custom mcpTools/mcpPrompts/mcpResources registered after the initial walk and — with no schema event — never surfaced. Resources gains a monotonic registrationVersion (bumped on set/delete) and the MCP transport rebuilds lazily per request when the registry moved (integer compare otherwise). This was reproduced by the integration test before the fix.

Where to look

  • The freshness mechanism is the most cross-cutting piece: a version bump in resources/Resources.ts set/delete and a per-request check in transport.handleMcpRequest. registerApplicationTools is synchronous and idempotent; the version snapshot is taken before the walk so mid-walk registrations re-walk on the next request.
  • Custom-match-first in readResource: registered author URIs win over the discovered surface. Reserved-scheme rejection at registration keeps built-ins unshadowable.
  • guessAppHttpUrlPrefix() produced a malformed authority when the port config used the host:port bind form (e.g. harper+rest://127.0.0.2:127.0.0.2:9926) — pre-existing, surfaced by runtime verification, fixed here (normalizePortForUrl) at the user’s request and verified live.

Testing

36 new unit tests (registry template compilation/matching/completions; resources surface incl. blob/JSON/error normalization and scheme back-compat; registration-walk validation incl. reserved schemes and live-class dispatch) + 7 integration tests on a real instance with a tableless docs fixture (list/templates/read text+blob+template, sanitized author errors, completion). All 436 MCP unit tests and the sdk/row-authz/sse-listchanged integration suites pass — no regressions from the scheme change. test:unit:main handed to CI (local dev instance holds the DB lock).

Generated by an LLM (Claude Fable 5), cross-reviewed by Codex (subscribe-scheme + template-diff regressions caught and fixed) and Gemini (freshness undefined-version loop + reserved-scheme shadowing caught and fixed); the final Codex pass caught two more (lazy rebuilds not emitting tools/list_changed; a parameterized-scheme bypass of the reserved-scheme guard), both fixed in 3af972e.

Docs: companion documentation PR — HarperFast/documentation#567.

Component authors can now expose arbitrary text/blob content under
author-chosen URIs on the application profile, parallel to mcpTools
(#622) and mcpPrompts: entries declare a fixed uri or an RFC-6570-style
uriTemplate ({name} one segment, {+name} cross-segment), served by an
instance method dispatched on the LIVE registry class, with optional
per-parameter completion values and resources list_changed support.

Exported-Resource descriptors now list under harper+rest:// (the spec
reserves https:// for web-fetchable resources); legacy http(s):// URIs
still read and subscribe. Reserved schemes are rejected in mcpResources
declarations so authors cannot shadow the built-in surfaces.

Also fixes a latent registration gap: component entry loading is
asynchronous past the boot awaits, so tableless components' custom
mcpTools/mcpPrompts/mcpResources could register after the initial walk
with no schema event to refresh it. Resources gains a monotonic
registrationVersion and the MCP transport rebuilds lazily per request
when the registry moved.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements MCP custom-resource registries, enabling component authors to publish custom content resources under custom URIs or URI templates. It also transitions exported-Resource descriptors to use a new harper+rest:// scheme instead of https:// for better spec compliance, while maintaining backward compatibility. Feedback on the changes highlights a potential issue with duplicate parameter names in URI templates that should be validated, and a bug in the unit tests where the legacy URI compatibility test incorrectly uses the new scheme instead of https://.

Comment thread components/mcp/customResourceRegistry.ts Outdated
Comment thread unitTests/components/mcp/resources.test.js
@claude

claude Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

nizzlenitz and others added 3 commits July 6, 2026 08:28
…om scheme

Codex final review: the lazy per-request rebuild could change the tool
set without emitting notifications/tools/list_changed (only prompts and
resources were notified), and the reserved-scheme guard could be
bypassed with a parameterized scheme position like {scheme}://{+path}.
The notifier per-session diffs, so no-op rebuilds stay silent; schemes
must now be literal and non-reserved.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e a legacy URI

Gemini bot review: docs:///{name}/{name} silently overwrote the first
captured value; and the scheme back-compat test requested harper+rest
for both cases (an overzealous scheme-migration sed), so the legacy
http(s) path was untested.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
guessAppHttpUrlPrefix appended the raw HTTP_PORT/HTTP_SECUREPORT config
value as if it were a bare port, producing authorities like
harper+rest://127.0.0.9:127.0.0.9:9926 when the port was configured as
a bind address (host:port). Extract just the port. Pre-existing (legacy
https:// descriptors were equally malformed); surfaced by runtime
verification of #1609 and rolled into this PR at the user's request.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment thread unitTests/components/mcp/customResourceRegistry.test.js Outdated
@kylebernhardy

Copy link
Copy Markdown
Member Author

This nails the functional shape we filed #1609 for — the mcp-custom-resources fixture (docs:///{+path} → a tableless readPage returning { text, mimeType: 'text/markdown' }) is exactly the docs-server case, and the across-segments template read + blob + completions all look right. Thanks for the fast turnaround.

One thing to lock down for that use case, since #1609's premise is a public docs server — i.e. anonymous / unauthenticated clients:

  • The code path looks correct for anonymous today: listCustomResources / matchCustomResource are keyed on profile only (no per-user filter), and readCustomResource passes user through and delegates RBAC to the method — parity with mcpTools, which we verified serves anonymous sessions (visibleTo: () => true).
  • But all 7 integration tests authenticate (auth = basicAuth(admin.username, admin.password)), so the anonymous path is never exercised. That's the one property our use case actually depends on.
  • The registry comment (customResourceRegistry.ts: "listed to every authenticated user on the profile") also reads as if auth is required, which — if the intent is that anonymous works — is worth tightening so operators don't assume a login gate.

Could you confirm anonymous resources/list + resources/read of a custom mcpResource is intended to work, and add one no-auth assertion to custom-resources.test.ts (drop the authorization header, expect the docs:///… read to succeed)? That would guarantee the public-docs scenario the issue is built around, and mirror how mcpTools already behaves.

nizzlenitz and others added 2 commits July 6, 2026 09:17
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Custom resources impose no auth of their own (listing is per-profile,
reads delegate to the author's method — parity with mcpTools'
visibleTo: () => true). The new integration case runs a full
no-Authorization-header session end-to-end; the registry doc no longer
implies a login gate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kylebernhardy

Copy link
Copy Markdown
Member Author

Confirmed: anonymous resources/list + resources/read of custom mcpResources is intended and structurally guaranteed — listing is keyed on profile only (no per-user filter), and reads pass the (possibly absent) user straight through to the author's method, exactly mirroring mcpTools' visibleTo: () => true posture. effectiveUser in the transport tolerates a userless request, so nothing in the new layer assumes a login.

Addressed in 0a9083e:

  • New integration case runs a full no-Authorization-header session (initialize → list → template read) and asserts the docs:///… content serves — on the harness instance the local mapping supplies the user the way a public deployment's anonymous mapping would, so the assertion pins "no auth header required at the MCP layer" end-to-end.
  • The registry doc comment no longer says "every authenticated user" — it now names the anonymous public-docs case explicitly so operators don't infer a login gate.

Comment generated by an LLM (Claude Fable 5).

@kylebernhardy kylebernhardy marked this pull request as ready for review July 7, 2026 00:40
kylebernhardy pushed a commit that referenced this pull request Jul 7, 2026
…1610)

Session-scoped buckets are trivially cycled by an anonymous client
(initialize, call, drop session, repeat) and reset on restart — no cost
control for public unauthenticated tools. Two additions, both opt-in:

- Per-client-identity token buckets (rateLimit.ts): keyed on client
  identity (socket IP, or the first value of a config-named trusted
  header for proxied deployments), surviving session cycling. Config
  perClientPerSecond/perClientBurst per profile, default off; burst
  floors at one whole token so fractional sustained rates still admit;
  per-profile config now cached with a 10s TTL to keep the per-call
  identity resolution off the env-lookup path; startup warning when an
  identity header is configured (spoofing trap).

- Durable quota hook (quota.ts): mcp.<profile>.quota.resource names an
  exported Resource whose static method (default allowMcpCall) is
  called per admitted tools/call with {identity, tool, user, profile,
  sessionId}; deny with {allowed:false, message?, retryAfterSeconds?}
  surfacing as kind:'quota_exceeded'. Fail-closed on misconfiguration
  or hook errors (#1422 precedent); dispatches on the live registry
  class; runs after the in-memory admit so throttled clients cannot
  spam a table-backed hook.

Stacked on feat/mcp-custom-resources-1609 for the lazy registry
freshness (#1613) that makes custom-tool fixtures deterministic.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kylebernhardy pushed a commit that referenced this pull request Jul 7, 2026
The rebase onto the #1613 branch had replayed the intervening v5.1
release commits (including the #1575 revert) as head-side commits,
polluting the PR diff; the branch now carries only this feature's
commit. The misconfiguration warn-once flag is per profile so a second
misconfigured profile still logs (gemini bot).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kriszyp

kriszyp commented Jul 7, 2026

Copy link
Copy Markdown
Member

Reviewed via Claude review-queue (full review: harper-1613-0a9083e.md). Solid third leg of the MCP custom-surface triad (mcpResources, after mcpTools/mcpPrompts) — the reserved-scheme guard, RBAC delegation posture, error sanitization, and lazy freshness-rebuild fix are all well done and well tested.

One real gap worth a look: {name} (single-segment) template params don't actually enforce "one path segment" once percent-decoding happens. compileUriTemplate builds {name} as ([^/]+) matched against the raw, still-encoded URI, then decodes the captured group afterward — so a client can encode / as %2F to defeat the segment boundary. Verified locally: docs:///{name} matched against docs:///..%2F..%2Fetc%2Fpasswd passes the regex (no literal /), then decodes to ../../etc/passwd. The module doc and tests explicitly frame {name} as the segment-safe alternative to {+name} — an author trusting that contract for filesystem/resource path construction (e.g. docs:///{id}fs.readFile(path.join(baseDir, params.id))) would be exposed to directory traversal. Doesn't bite the shipped fixture (readPage does an exact-key dict lookup, and only uses {+path} anyway), so no live exploit here — but the primitive is reusable by any future/customer app.

Suggested fix: validate the raw captured segment has no %2F/%5C before decoding, or split-then-decode per already-delimited segment. At minimum, document the caveat so authors know {name} is a shape hint, not a security boundary.

{name} captures ran against the still-encoded URI, so %2F/%5C slipped
through the [^/]+ segment class and decoded to real separators after
the boundary check — defeating the one-segment contract authors rely
on for path construction (kriszyp review). Such URIs now simply fail
to match; {+name} reserved expansion still decodes them by design.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kylebernhardy

Copy link
Copy Markdown
Member Author

Great catch — fixed in a49b8b1. compileUriTemplate now records per-param reserved flags, and matchCustomResource rejects any {name} capture whose RAW text contains %2F/%5C (case-insensitive) before decoding — the URI simply fails to match a single-segment slot, so the one-segment contract holds through percent-decoding. {+name} reserved expansion still decodes encoded separators by design. Verified your exact vector: docs:///{name} vs docs:///..%2F..%2Fetc%2Fpasswd no longer matches (unit test added, plus %5C, lowercase %2f, an ordinary %20-in-{name} control, and a {+path} still-decodes control).

Comment generated by an LLM (Claude Fable 5).

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed via Claude review-queue — the {name} decode-order path-traversal fix is confirmed and empirically re-verified. CI red is a pre-existing unrelated main issue. LGTM.

@kriszyp kriszyp merged commit 0ac6ce3 into v5.1 Jul 7, 2026
43 of 46 checks passed
@kriszyp kriszyp deleted the feat/mcp-custom-resources-1609 branch July 7, 2026 23:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants